Skip to content

Conversation

@WaterWhisperer
Copy link
Contributor

@WaterWhisperer WaterWhisperer commented Jan 2, 2026

Which Issue(s) This PR Fixes(Closes)

Fixes #5326

Brief Description

How Did You Test This Change?

Summary by CodeRabbit

  • Refactor
    • Optimized internal broker runtime components to enhance code efficiency and maintainability.

✏️ Tip: You can customize this high-level summary in your review settings.

@rocketmq-rust-bot
Copy link
Collaborator

🔊@WaterWhisperer 🚀Thanks for your contribution🎉!

💡CodeRabbit(AI) will review your code first🔥!

Note

🚨The code review suggestions from CodeRabbit are to be used as a reference only, and the PR submitter can decide whether to make changes based on their own judgment. Ultimately, the project management personnel will conduct the final code review💥.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 2, 2026

Walkthrough

The change refactors the public API of BrokerRuntimeInner<MS> by updating the broker_stats accessor method's return type from &Option<BrokerStats<MS>> to Option<&BrokerStats<MS>>, implemented via as_ref(). This change occurs in two locations within the same file.

Changes

Cohort / File(s) Change Summary
API Accessor Refactoring
rocketmq-broker/src/broker_runtime.rs
Updated broker_stats(&self) method signature in two locations: return type changed from &Option<BrokerStats<MS>> to Option<&BrokerStats<MS>>, implementation changed from direct reference to self.broker_stats.as_ref()

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A hop and a bound, the API's improved,
No more nested refs causing brokers to grieve,
Option<&T> flows more smoothly than before,
Idiomatic Rust opens each door,
Small changes, big clarity—this rabbit approves! ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: improving the broker_stats accessor to return Option<&T> instead of &Option.
Linked Issues check ✅ Passed The pull request successfully implements the requirement from issue #5326 to change the broker_stats accessor return type from &Option<BrokerStats> to Option<&BrokerStats>.
Out of Scope Changes check ✅ Passed All changes in the pull request are directly related to modifying the broker_stats accessor method signature as specified in issue #5326, with no extraneous modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 80e75e2 and 515fd61.

📒 Files selected for processing (1)
  • rocketmq-broker/src/broker_runtime.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Build & Test (windows-latest)
  • GitHub Check: Build & Test (macos-latest)
  • GitHub Check: Build & Test (ubuntu-latest)
  • GitHub Check: Code Coverage
  • GitHub Check: auto-approve
🔇 Additional comments (1)
rocketmq-broker/src/broker_runtime.rs (1)

1752-1754: LGTM! More idiomatic Rust API.

The change from &Option<BrokerStats<MS>> to Option<&BrokerStats<MS>> is a better API design that follows Rust conventions. Callers can now pattern-match directly without dealing with nested references.

All call sites have been properly updated and are compatible with the new signature.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link

Summary of Changes

Hello @WaterWhisperer, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refines the API for accessing broker statistics within the BrokerRuntimeInner struct. By adjusting the broker_stats method to return an Option containing a reference to the BrokerStats object, it enhances the usability and idiomatic nature of the Rust codebase, aligning with common patterns for handling optional data and improving type safety.

Highlights

  • API Ergonomics Improvement: The broker_stats accessor method in BrokerRuntimeInner has been updated to return Option<&BrokerStats<MS>> instead of &Option<BrokerStats<MS>>. This change makes the API more idiomatic and convenient for Rust developers, allowing direct access to a reference of BrokerStats<MS> when present.
  • Reference Handling: The implementation now uses self.broker_stats.as_ref(), which correctly converts Option<T> to Option<&T>, aligning with best practices for working with optional references in Rust.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request improves the broker_stats accessor method in BrokerRuntimeInner by changing its return type from &Option<BrokerStats<MS>> to the more idiomatic Option<&BrokerStats<MS>>. This is an excellent change that improves the ergonomics for callers of this function by allowing direct use of Option's combinator methods. The implementation using self.broker_stats.as_ref() is correct and follows Rust best practices. While reviewing, I noticed other similar accessor methods in BrokerRuntimeInner that could benefit from the same change (e.g., replicas_manager, timer_message_store). A follow-up pull request to make these consistent would be a great addition. This change is a definite improvement, and I'm happy to approve it.

@codecov
Copy link

codecov bot commented Jan 2, 2026

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.11%. Comparing base (80e75e2) to head (515fd61).
⚠️ Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
rocketmq-broker/src/broker_runtime.rs 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5351   +/-   ##
=======================================
  Coverage   37.11%   37.11%           
=======================================
  Files         798      798           
  Lines      107786   107786           
=======================================
  Hits        40007    40007           
  Misses      67779    67779           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Collaborator

@rocketmq-rust-bot rocketmq-rust-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - All CI checks passed ✅

@mxsm mxsm merged commit 853d39a into mxsm:main Jan 2, 2026
11 of 22 checks passed
@rocketmq-rust-bot rocketmq-rust-bot added approved PR has approved and removed ready to review waiting-review waiting review this PR labels Jan 2, 2026
@WaterWhisperer WaterWhisperer deleted the enhance-5326 branch January 2, 2026 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Enhancement✨] Improve broker_stats accessor to return Option<&T> in BrokerRuntimeInner

4 participants